Skip to content

Conversation

@alyssajoyner
Copy link
Contributor

@alyssajoyner alyssajoyner commented Oct 21, 2025

This PR removes deprecated code, components and styles.

Code

  • toBeCalledTimes is deprecated; replaced with toHaveBeenCalledTimes
  • toBeCalledWith is deprecated; replaced with toHaveBeenCalledWith
  • lightTheme is deprecated; replaced with theme
  • error is deprecated; replaced with errors
  • MutableDataFrame is deprecated; replaced with DataFrame
  • getDataProvider is deprecated; replaced with getSupplementaryQueryRequest
  • condition is deprecated

Components

  • HorizontalGroup and VerticalGroup are deprecated; replaced with Stack

Styles

  • Remove gf-form styles

NOTE: Select component is deprecated and should be replaced with Combobox. This work is not included in this PR due to testing issues with Combobox.

@alyssajoyner alyssajoyner marked this pull request as ready for review October 21, 2025 21:21
@alyssajoyner alyssajoyner requested a review from a team as a code owner October 21, 2025 21:21
@alyssajoyner alyssajoyner moved this from Incoming to Needs Review in Partner Datasources Oct 21, 2025
@alyssajoyner alyssajoyner moved this from Needs Review to Incoming in Partner Datasources Oct 22, 2025
@alyssajoyner alyssajoyner marked this pull request as draft October 22, 2025 03:52
@adamyeats adamyeats moved this from Incoming to In Progress in Partner Datasources Oct 22, 2025
@alyssajoyner alyssajoyner moved this from In Progress to Needs Review in Partner Datasources Oct 23, 2025
@alyssajoyner alyssajoyner marked this pull request as ready for review October 23, 2025 18:20
Comment on lines +200 to +207
{/* <Switch
value={builderState.liveView}
onChange={onOptionChange('liveView')}
label={labels.liveView.label}
tooltip={labels.liveView.tooltip}
inline
/> */}
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this?

</InlineFormLabel>
<Input
width={200}
// width={200}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this?

/>
</div>
<div className="gf-form">
<div>
Copy link
Collaborator

@bossinc bossinc Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we able to remove the divs without styling in this file?

Comment on lines +205 to +208
// REMOVE this whole method
// getDataProvider(type: SupplementaryQueryType, request: DataQueryRequest<CHQuery>): Observable<DataQueryResponse> | undefined { ... }

// ADD this instead:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we didnt remove a function called getDataProvider right?

Do we need to keep this comment even if we did?

const key = escapeKey(f.key);
const value = escapeValueBasedOnOperator(f.value, f.operator);
const condition = i !== adHocFilters.length - 1 ? (f.condition ? f.condition : 'AND') : '';
const condition = i !== adHocFilters.length - 1 ? 'AND' : '';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this always make adHocFileters AND so you will never be able to use OR

values: field.values,
const effectiveName = oneLevelDetected && field.name === DEFAULT_LOGS_ALIAS ? 'logs' : field.name;

const logLevel = LogLevel[effectiveName as keyof typeof LogLevel] ?? LogLevel.unknown;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this to ?? changes the behavor. Is this what we want?

validateSql: {
label: 'Validate SQL',
tooltip: 'Validate SQL in the editor.',
tooltip: 'Validate SQL in the editor',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe tool tips that are sentences have periods at the end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

2 participants